-
-
Notifications
You must be signed in to change notification settings - Fork 4k
TilemapChunk single quad; TileData (color/visibility) #19924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
TilemapChunk single quad; TileData (color/visibility) #19924
Conversation
058304a
to
edbc343
Compare
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
edbc343
to
5e026ef
Compare
aa512c7
to
606bcb4
Compare
606bcb4
to
e9c1e1a
Compare
crates/bevy_sprite/src/tilemap_chunk/tilemap_chunk_material.wgsl
Outdated
Show resolved
Hide resolved
@@ -60,10 +54,36 @@ pub struct TilemapChunk { | |||
pub alpha_mode: AlphaMode2d, | |||
} | |||
|
|||
#[derive(Clone, Copy, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc strings please.
} | ||
|
||
impl TileData { | ||
pub fn from_index(index: u16) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc strings, ideally linking to where you get this index from.
fn default() -> Self { | ||
Self { | ||
tileset_index: 0, | ||
color: Color::WHITE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a default of white rather than black or our standard missing data magenta?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think white is the standard default for tinting, since you're effectively leaving the original pixel color untouched. Maybe it makes sense for it to be optional though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core idea is good, but needs more docs :) The default color choice is classic bikeshedding; feel free to leave it as white if you think that's best.
Once that's done I'll test it locally then approve.
Objective
Testing